doc_suspicious_footnotes: lint text that looks like a footnote #14708
Conversation
rustbot
added
the
S-waiting-on-review
notriddle
force-pushed
the
doc_suspicious_footnotes
branch
from
9f74df2 to
c56ef28
Compare
notriddle
force-pushed
the
doc_suspicious_footnotes
branch
from
c56ef28 to
1356e76
Compare
|
Looks good to me, thanks! r? samueltardieu |
|
samueltardieu is on vacation. Please choose another assignee. |
| /// | ||
| /// This is a footnote[^2]. | ||
| /// | ||
| /// [^2]: hello world |
There was a problem hiding this comment.
Could this also check for #![doc], //!, #[doc] and the include!s macros?
rustbot
added
S-waiting-on-author
|
@blyxyas Okay, I've implemented variants on all your suggestions. |
notriddle
force-pushed
the
doc_suspicious_footnotes
branch
from
b0e3325 to
99b16ae
Compare
| let start_of_md_line = doc.as_bytes()[..start] | ||
| .iter() | ||
| .rposition(|&c| c == b'\n' || c == b'\r') | ||
| .unwrap_or(0); | ||
| let end_of_md_line = doc.as_bytes()[start..] | ||
| .iter() | ||
| .position(|&c| c == b'\n' || c == b'\r') | ||
| .unwrap_or(doc.len() - start) | ||
| + start; | ||
| let span_md_line = fragments | ||
| .span(cx, start_of_md_line..end_of_md_line) | ||
| .unwrap_or(this_fragment.span); | ||
| let span_whole_line = cx.sess().source_map().span_extend_to_line(span_md_line); | ||
| if let Ok(mut pfx) = cx | ||
| .sess() | ||
| .source_map() | ||
| .span_to_snippet(span_whole_line.until(span_md_line)) | ||
| && let Ok(mut sfx) = cx | ||
| .sess() | ||
| .source_map() | ||
| .span_to_snippet(span_md_line.shrink_to_hi().until(span_whole_line.shrink_to_hi())) | ||
| { | ||
| let mut insert_before = String::new(); | ||
| let mut insert_after = String::new(); | ||
| let span = if this_fragment.kind == rustc_resolve::rustdoc::DocFragmentKind::RawDoc | ||
| && (!pfx.is_empty() || !sfx.is_empty()) | ||
| { | ||
| if (pfx.trim() == "#[doc=" || pfx.trim() == "#![doc=") && sfx.trim() == "]" { | ||
| // try to use per-line doc fragments if that's what the author did | ||
| pfx.push('"'); | ||
| sfx.insert(0, '"'); | ||
| span_whole_line.shrink_to_hi() | ||
| } else { | ||
| // otherwise, replace the whole line with the result | ||
| pfx = String::new(); | ||
| sfx = String::new(); | ||
| insert_before = format!(r#"r###"{}"#, this_fragment.doc); | ||
| r####""###"####.clone_into(&mut insert_after); | ||
| span_md_line | ||
| } | ||
| } else { | ||
| span_whole_line.shrink_to_hi() | ||
| }; |
There was a problem hiding this comment.
Understanding this is proving a little bit hard, but for finding where to put the new comment, I'm thinking that you could loop over the atributes of the item with tcx.hir().attrs(), finding the last one that fits is_doc_comment and then adding our suggestion to the end of that one (on a new line)
Also, I'm not sure how does this interact with cfg and cfg_attr, we should probably test that =^w^=
| @@ -1147,7 +1186,8 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize | |||
| // Don't check the text associated with external URLs | |||
| continue; | |||
| } | |||
| text_to_check.push((text, range, code_level)); | |||
| text_to_check.push((text, range.clone(), code_level)); | |||
| doc_suspicious_footnotes::check(cx, doc, range, &fragments); | |||
There was a problem hiding this comment.
I'm sure that you've already tried this, but is there a reason that you didn't use FootnoteReference from ķEvent? You would not have to do span magic and manually parse footnotes and it seems that this function doesn't actually use text
There was a problem hiding this comment.
The FootnoteReference event is only emitted if the code is actually a footnote. This lint fires for text that looks like a footnote reference but isn't.
changelog: [
doc_suspicious_footnotes]: lint for text that looks like a footnote reference but has no definitionThis is an alternative to rust-lang/rust#137803, meant to address the concerns about false positives.
This lint only fires when the apparent footnote reference has a name that's made from pure ASCII digits. This choice is justified by running lintcheck on the top 200 crates, plus the clippy default set:
cargouses one in its job_queue module, and that's all it found.cc @GuillaumeGomez